Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 5, 2025

The WilliamsR implementation had build errors and lacked consistency across series, buffer, and stream styles. The StreamHub test also needed comprehensive rollback validation following the canonical EMA/ADX pattern.

Changes

Build Error Fixes

  • WilliamsR.StreamHub.cs
    • Fixed type mismatch: Changed RollingWindowMax/Min<decimal> to RollingWindowMax/Min<double>
    • Fixed undefined variable: Changed l to low
    • Removed unused variable o (open)
    • Fixed typo: Nan2Null()NaN2Null() in BufferList

Consistency Improvements

Implemented consistent composite lookback buffer pattern across all three styles:

  • Series: Already uses CalcStoch internally (no changes needed)
  • BufferList: Now delegates to StochList internally
    • Eliminates duplicate HLC buffer management code
    • Uses Fast Stochastic parameters (lookback, signal=1, smooth=1)
    • Converts Stochastic %K to Williams %R by subtracting 100
  • StreamHub: Maintains direct O(1) RollingWindow implementation for performance
    • Added comment clarifying relationship to Stochastic formula
    • Uses same efficient window pattern as StochHub

All three styles now consistently express: Williams %R = Stochastic %K - 100

Enhanced StreamHub Test

  • Enhanced QuoteObserver_WithWarmupLateArrivalAndRemoval_MatchesSeriesExactly()
    • Prefill 20 quotes before observer initialization (warmup coverage)
    • Skip quote at index 80, stream remaining quotes
    • Resend duplicates at indices 101-104
    • Insert skipped quote and verify against original series
    • Remove quote using base class removeAtIndex constant (495), not hardcoded value
    • Compare against RevisedQuotes from base class
    • Add strict ordering verification with BeEquivalentTo(..., o => o.WithStrictOrdering())
    • Add proper cleanup (Unsubscribe(), EndTransmission())

Test Pattern

Now matches canonical EMA/ADX StreamHub test structure:

// Prefill warmup
quoteHub.Add(Quotes.Take(20));
WilliamsRHub observer = quoteHub.ToWilliamsRHub(lookbackPeriods);

// Stream with skip and duplicates
for (int i = 20; i < length; i++) {
    if (i == 80) { continue; }
    quoteHub.Add(Quotes[i]);
    if (i is > 100 and < 105) { quoteHub.Add(Quotes[i]); }
}

// Validate Insert/Remove operations
quoteHub.Insert(Quotes[80]);
actuals.Should().BeEquivalentTo(expectedOriginal, o => o.WithStrictOrdering());

quoteHub.Remove(Quotes[removeAtIndex]);
actuals.Should().BeEquivalentTo(RevisedQuotes.ToWilliamsR(lookbackPeriods), o => o.WithStrictOrdering());

Verification

  • ✅ Build succeeds (0 errors, 0 warnings)
  • ✅ All tests pass (2078 tests)
  • ✅ Code formatting verified
  • ✅ Markdown linting passes

Addresses follow-on task T177 from #1723.

Original prompt

This section details on the original issue you should resolve

<issue_title>add rollback validation tests for WilliamsR StreamHub</issue_title>
<issue_description>## Problem

The WilliamsR StreamHub indicator currently has basic rollback validation but doesn't follow the full comprehensive pattern established for other indicators. The test should be enhanced to match the canonical EMA/ADX pattern.

Required Test Cases

The WilliamsR QuoteObserver test should include (following the canonical EMA/ADX pattern):

  1. Prefill warmup: Add 20-30 quotes before observer initialization
  2. Streaming with skip: Stream remaining quotes, skip quote at index 80
  3. Duplicate handling: Resend duplicates for indices 101-104
  4. Late arrival (Insert): Insert the skipped quote and verify Series parity
  5. Removal: Remove quote at removeAtIndex constant (495) from base class, not hardcoded value
  6. Revised series parity: Use RevisedQuotes from base class for comparison
  7. Strict ordering: Use BeEquivalentTo(series, o => o.WithStrictOrdering())
  8. Cleanup: Call Unsubscribe() and EndTransmission()

Key Requirements

  • Must use base class removeAtIndex constant (495) for consistency across tests
  • Must use base class RevisedQuotes for revised series comparison
  • Should follow exact pattern from tests/indicators/e-k/Ema/Ema.StreamHub.Tests.cs

Reference

  • Project task: T177 (Comprehensive Rollback Validation)
  • Task file: .specify/specs/001-develop-streaming-indicators/tasks.md
  • Canonical pattern: tests/indicators/e-k/Ema/Ema.StreamHub.Tests.cs QuoteObserver method
  • Current test: tests/indicators/s-z/WilliamsR/WilliamsR.StreamHub.Tests.cs
  • Implementation: src/s-z/WilliamsR/WilliamsR.StreamHub.cs

Success Criteria

  • All comprehensive rollback test cases pass
  • Uses base class constants (removeAtIndex, RevisedQuotes) consistently
  • Follows exact pattern from canonical EMA test
  • No hardcoded indices or test data

This is a follow-on task from #1723 </issue_description>

Comments on the Issue (you are @copilot in this section)

Custom agent used: streamhub
Expert guidance for StreamHub indicator development - implementation patterns, provider selection, state management, and real-time processing


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

The test file for WilliamsR StreamHub has been restructured to implement comprehensive rollback validation. The modifications establish a warmup phase using prefilled quotes, defer observer creation, then progressively add quotes with simulated streaming conditions including skipped indices and duplicate re-entries. A late-arrival quote is inserted, streaming results are validated against batch-computed expectations, followed by a deletion step with revised series comparison. The test concludes with cleanup operations.

Assessment against linked issues

Objective Addressed Explanation
Prefill warmup (add 20-30 quotes before observer initialization) and streaming with skip, duplicate handling, late arrival validation (#1725)
Removal using base class removeAtIndex constant and revised series parity using base class RevisedQuotes (#1725) The summary references a "deletion step" and "revised expected series" but does not specify whether the removeAtIndex constant from the base class is used or if the value is hardcoded, and whether RevisedQuotes from the base class is referenced.
Strict ordering assertion using BeEquivalentTo(series, o => o.WithStrictOrdering()) and cleanup with Unsubscribe() and EndTransmission() (#1725) The summary confirms cleanup operations are present but does not specify the exact assertion syntax used for result comparison.

Possibly related PRs


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Enhanced QuoteObserver_WithWarmupLateArrivalAndRemoval_MatchesSeriesExactly() test to follow canonical EMA/ADX pattern with:
- Warmup prefill (20 quotes before observer init)
- Skip/late arrival (Insert) at index 80
- Duplicate handling for indices 101-104
- Removal using base class removeAtIndex constant (495)
- Revised series comparison using RevisedQuotes from base class
- Strict ordering verification with BeEquivalentTo
- Proper cleanup with Unsubscribe() and EndTransmission()

All test cases pass and match the canonical pattern from EMA/ADX StreamHub tests.

Co-authored-by: DaveSkender <[email protected]>
Copilot AI changed the title [WIP] Add rollback validation tests for WilliamsR StreamHub Add comprehensive rollback validation for WilliamsR StreamHub Nov 5, 2025
Copilot AI requested a review from DaveSkender November 5, 2025 16:16
@DaveSkender DaveSkender changed the title Add comprehensive rollback validation for WilliamsR StreamHub test: WilliamsR StreamHub late arrival tests Nov 5, 2025
@DaveSkender DaveSkender marked this pull request as ready for review November 5, 2025 16:53
@DaveSkender
Copy link
Owner

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

✅ Actions performed

Full review triggered.

@DaveSkender DaveSkender changed the base branch from v3 to fix-misc December 8, 2025 08:57
Copy link
Owner

@DaveSkender DaveSkender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot fix the underlying implementation of stoch and WilliamsR to use a consistent composite lookback buffer or some kind of consistency across series, buffer, and stream hub implementations. Finish by resolving all code completion checklist items.

Williams %R is mathematically equivalent to Fast Stochastic %K - 100.
Updated WilliamsRList to delegate to StochList for consistent composite
lookback buffer implementation across series, buffer, and stream styles.

- Simplifies WilliamsRList implementation
- Eliminates duplicate HLC buffer logic
- Maintains same mathematical results via StochList delegation
- StreamHub keeps direct implementation for performance (O(1) RollingWindows)

Co-authored-by: DaveSkender <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 8, 2025

@copilot fix the underlying implementation of stoch and WilliamsR to use a consistent composite lookback buffer or some kind of consistency across series, buffer, and stream hub implementations. Fini...

Fixed compilation errors and implemented consistent composite lookback buffer for WilliamsR and Stoch:

Build errors fixed:

  • Changed RollingWindowMax/Min from decimal to double in WilliamsRHub
  • Fixed undefined variable llow
  • Removed unused variable o
  • Fixed typo Nan2NullNaN2Null in BufferList

Consistency improvements:

  • Series: Already uses CalcStoch internally (no changes)
  • BufferList: Now delegates to StochList - eliminates duplicate HLC buffer logic
  • StreamHub: Maintains direct O(1) RollingWindow implementation for performance, with comment clarifying Stochastic relationship

All three styles now consistently express: Williams %R = Stochastic %K - 100

Code completion checklist complete:
✅ Build succeeds (0 errors, 0 warnings)
✅ All tests pass (2078 passed)
✅ Code formatting verified
✅ Markdown linting passes

Commit: c1e5b36

Copilot AI changed the title test: WilliamsR StreamHub late arrival tests fix: WilliamsR implementation consistency and StreamHub rollback tests Dec 8, 2025
Copilot AI requested a review from DaveSkender December 8, 2025 09:22
@DaveSkender DaveSkender merged commit a053c8f into fix-misc Dec 8, 2025
7 checks passed
@DaveSkender DaveSkender deleted the copilot/add-rollback-validation-tests branch December 8, 2025 10:37
@github-project-automation github-project-automation bot moved this from In Review to Done in Stock Indicators for .NET Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

add rollback validation tests for WilliamsR StreamHub

2 participants